Repository navigation
Ask access(X_OK) whether an overlay PATH match may run, on every target [patch] - #141
Open
matt-edmondson wants to merge 2 commits into
Open
matt-edmondson wants to merge 2 commits into
matt-edmondson wants to merge 2 commits into
Conversation
…et [patch] #137 skipped a file on the overlay PATH that had no execute bit, but the check used File.GetUnixFileMode and sat under NET7_0_OR_GREATER. The net5.0, net6.0 and netstandard builds accepted any file that existed, so on Linux and macOS a non-executable file earlier on the overlay PATH won and the start failed with "Permission denied" instead of running the executable copy later on. The tests only target net10.0, so they never saw it. TryExecutable now calls libc access(path, X_OK) through a new NativeMethods class on every target outside Windows, so all builds share one check and the net10.0 tests exercise the code the older builds run. access answers for the caller, as a shell's PATH search does, so it also passes over a file only its group or others may execute, which "any execute bit" accepted. If libc cannot be called the file is kept, as before the check existed. New tests: a file whose owner (the caller) lacks the execute bit while group and others have it is skipped in favour of the executable copy (inconclusive as root, which may run it), and CanExecute reports a 755 file as runnable and a 644 file as not. Fixes #139 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017vfJW5DyciGEV4wZvik3pQ
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
|
|
||
| [DefaultDllImportSearchPaths(DllImportSearchPath.SafeDirectories)] | ||
| [DllImport("libc", EntryPoint = "access", CharSet = CharSet.Ansi, BestFitMapping = false, ThrowOnUnmappableChar = true)] | ||
| private static extern int Access(string path, int mode); |
Contributor
Author
There was a problem hiding this comment.
Keeping the P/Invoke. The suggested fix would bring #139 back:
File.GetUnixFileModedoesn't exist on net5.0, net6.0 or netstandard2.0/2.1. Those are the builds net5.0/net6.0/netstandard builds pick a non-executable file earlier on the overlay PATH, so the command fails with "Permission denied" instead of running the executable copy later on #139 is about, and the reason the old check sat under#if NET7_0_OR_GREATERand those builds had no check at all.- "Any execute bit" is also the weaker test. It accepts a 0655 file the caller owns but may not execute.
ExecuteAsyncShouldSkipAFileOnTheOverlayPathThatOnlyOthersMayExecutefails with "Permission denied" when that check is put back (run as non-root).
access(X_OK) is the managed-free equivalent of what a shell's PATH search asks. The interop is one call in NativeMethods, with DefaultDllImportSearchPaths set. It falls back to keeping the candidate if libc can't be loaded, and it only runs off Windows. I'm leaving this thread open for the maintainer's call.
Generated by Claude Code
CanExecute's DllNotFoundException / EntryPointNotFoundException fallback had no test, so SonarCloud's new-code coverage was 60% (gate is 80%). The access call is now a parameter of an internal overload, as TryKill's kill is, and a test makes it throw both exceptions and checks the file is kept. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017vfJW5DyciGEV4wZvik3pQ
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #139
What changed
#137 made the overlay-PATH search skip a file with no execute bit. That check used
File.GetUnixFileModeunder#if NET7_0_OR_GREATER, so the net5.0, net6.0 and netstandard builds accepted any file that existed. On Linux and macOS, a non-executable file earlier on the overlayPATHtherefore won, and the start failed with "Permission denied" instead of running the executable copy later on the path.TryExecutablenow calls libcaccess(path, X_OK)through a new internalNativeMethods.CanExecuteon every target outside Windows. All builds share this one check, so the net10.0 tests exercise the code the older builds run.accessanswers for the calling process, which is what a shell'sPATHsearch does. It also skips a file that only its group or others may execute. The old "any execute bit" test accepted that file, and starting it then failed.DllNotFoundException/EntryPointNotFoundException), the file is kept as a candidate. That matches the behaviour before the check existed.I left the README alone. Its sentence "On other platforms the file must have an execute bit" is accurate on every target once this lands. It is also the line #140 edits, so changing it here would make the two PRs conflict.
Tests
ExecuteAsyncShouldSkipAFileOnTheOverlayPathThatOnlyOthersMayExecute: the first file on the overlay path is mode 0655, so the caller owns it but may not execute it. The test expects the executable copy to run. It reports inconclusive when run as root, because root may run any file that has an execute bit.CanExecuteShouldReportWhetherTheCallerMayExecuteTheFile: a 755 file can run and a 644 file can't.Win32Exception ... Permission denied. With the fix, all 6 overlay/CanExecute tests passed.mainit fails withWin32Exception ... '/tmp/rc-noexec/rctool' ... Permission denied, and on this branch it printsexecutable copy.Generated by Claude Code